-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Pretty print Fn<(..., ...)>
trait refs with parentheses (almost) always
#118268
Conversation
r? @wesleywiser (rustbot has picked a reviewer for you, use r? to override) |
Some changes occurred in src/tools/clippy cc @rust-lang/clippy |
@@ -99,7 +99,7 @@ impl<'tcx> LateLintPass<'tcx> for FutureNotSend { | |||
db.note(format!( | |||
"`{}` doesn't implement `{}`", | |||
trait_pred.self_ty(), | |||
trait_pred.trait_ref.print_only_trait_path(), | |||
trait_pred.trait_ref.print_trait_sugared(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this doesn't matter -- can revert. I actually don't even know why this uses print_only_trait_path
, given that it's always Send
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's likely copy-pasted from a more general rustc error.
This comment was marked as outdated.
This comment was marked as outdated.
ef938e9
to
ecccf33
Compare
"ambiguous `{assoc_name}` from `{}`", | ||
bound.print_only_trait_path(), | ||
), | ||
format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared(),), | |
format!("ambiguous `{assoc_name}` from `{}`", bound.print_trait_sugared()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a lot of these :(
I ctrl+f'd for ,);
and ,),
yesterday and found like >50 instances.
compiler/rustc_trait_selection/src/traits/error_reporting/on_unimplemented.rs
Show resolved
Hide resolved
TraitRefPrintSugared<'tcx> { | ||
if !with_no_queries() | ||
&& let Some(kind) = cx.tcx().fn_trait_kind_from_def_id(self.0.def_id) | ||
&& let ty::Tuple(args) = self.0.args.type_at(1).kind() | ||
{ | ||
p!(write("{}", kind.as_str()), "("); | ||
for (i, arg) in args.iter().enumerate() { | ||
if i > 0 { | ||
p!(", "); | ||
} | ||
p!(print(arg)); | ||
} | ||
p!(")"); | ||
} else { | ||
p!(print_def_path(self.0.def_id, self.0.args)); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will never print the Output
, is that ok?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's no Output
that we have access to, unfortunately :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Didn't we hack something together for Future::Output
somewhere?
Edit: I think I was thinking of pretty_print_opaque_impl_type
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess I could eagerly look into the self type and see if it's something with a signature, but it's not always going to be possible. Let me see.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you manage it, great. If not, r=me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, when we extract this information from the self type, it's often not right. For example, when we have:
fn test<F: Fn(u32) -> u32>() {}
test(|_: i32| -> i32 { 0 });
Extracting the return type from the self type will give us an error message mentioning something like {closure}: Fn(u32) -> i32
, not Fn(u32) -> u32
.
@bors r=estebank |
…tebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.
…tebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.
…iaskrgr Rollup of 7 pull requests Successful merges: - rust-lang#118157 (Add `never_patterns` feature gate) - rust-lang#118191 (Suggest `let` or `==` on typo'd let-chain) - rust-lang#118231 (also add is_empty to const raw slices) - rust-lang#118333 (Print list of missing target features when calling a function with target features outside an unsafe block) - rust-lang#118426 (ConstProp: Correctly remove const if unknown value assigned to it.) - rust-lang#118428 (rustdoc: Move `AssocItemRender` and `RenderMode` to `html::render`.) - rust-lang#118438 (Update nto-qnx.md) Failed merges: - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) r? `@ghost` `@rustbot` modify labels: rollup
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message
|
ecccf33
to
f6c30b3
Compare
@bors r=estebank |
…tebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.
…tebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.
…tebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#117793 (Update variable name to fix `unused_variables` warning) - rust-lang#118123 (Add support for making lib features internal) - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) - rust-lang#118346 (Add `deeply_normalize_for_diagnostics`, use it in coherence) - rust-lang#118350 (Simplify Default for tuples) - rust-lang#118450 (Use OnceCell in cell module documentation) - rust-lang#118585 (Fix parser ICE when recovering `dyn`/`impl` after `for<...>`) - rust-lang#118587 (Cleanup error handlers some more) - rust-lang#118642 (bootstrap(builder.rs): Don't explicitly warn against `semicolon_in_expressions_from_macros`) r? `@ghost` `@rustbot` modify labels: rollup
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#117793 (Update variable name to fix `unused_variables` warning) - rust-lang#118123 (Add support for making lib features internal) - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) - rust-lang#118346 (Add `deeply_normalize_for_diagnostics`, use it in coherence) - rust-lang#118350 (Simplify Default for tuples) - rust-lang#118450 (Use OnceCell in cell module documentation) - rust-lang#118585 (Fix parser ICE when recovering `dyn`/`impl` after `for<...>`) - rust-lang#118587 (Cleanup error handlers some more) - rust-lang#118642 (bootstrap(builder.rs): Don't explicitly warn against `semicolon_in_expressions_from_macros`) r? `@ghost` `@rustbot` modify labels: rollup
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#117793 (Update variable name to fix `unused_variables` warning) - rust-lang#118123 (Add support for making lib features internal) - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) - rust-lang#118346 (Add `deeply_normalize_for_diagnostics`, use it in coherence) - rust-lang#118350 (Simplify Default for tuples) - rust-lang#118450 (Use OnceCell in cell module documentation) - rust-lang#118585 (Fix parser ICE when recovering `dyn`/`impl` after `for<...>`) - rust-lang#118587 (Cleanup error handlers some more) - rust-lang#118642 (bootstrap(builder.rs): Don't explicitly warn against `semicolon_in_expressions_from_macros`) r? `@ghost` `@rustbot` modify labels: rollup
…mpiler-errors Rollup of 9 pull requests Successful merges: - rust-lang#117793 (Update variable name to fix `unused_variables` warning) - rust-lang#118123 (Add support for making lib features internal) - rust-lang#118268 (Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always) - rust-lang#118346 (Add `deeply_normalize_for_diagnostics`, use it in coherence) - rust-lang#118350 (Simplify Default for tuples) - rust-lang#118450 (Use OnceCell in cell module documentation) - rust-lang#118585 (Fix parser ICE when recovering `dyn`/`impl` after `for<...>`) - rust-lang#118587 (Cleanup error handlers some more) - rust-lang#118642 (bootstrap(builder.rs): Don't explicitly warn against `semicolon_in_expressions_from_macros`) r? `@ghost` `@rustbot` modify labels: rollup
Rollup merge of rust-lang#118268 - compiler-errors:pretty-print, r=estebank Pretty print `Fn<(..., ...)>` trait refs with parentheses (almost) always It's almost always better, at least in diagnostics, to print `Fn(i32, u32)` instead of `Fn<(i32, u32)>`. Related to but doesn't fix rust-lang#118225. That needs a separate fix.
Actually, I wonder if we should print |
It's almost always better, at least in diagnostics, to print
Fn(i32, u32)
instead ofFn<(i32, u32)>
.Related to but doesn't fix #118225. That needs a separate fix.